fix: consolidate evicted entries into long-term memory#166
fix: consolidate evicted entries into long-term memory#166
Conversation
STLTMemory._process_step_core() was discarding popped entries and then summarizing the *remaining* short-term memories instead of the evicted ones. This caused permanent data loss — old memories were deleted without ever being integrated into long-term memory. Changes: - _process_step_core now captures all evicted entries and returns them alongside the new entry - Pop consolidation_capacity entries (not just one) when triggered - _build_consolidation_prompt accepts evicted entries and asks the LLM to integrate them into the existing long-term summary - _update_long_term_memory and _aupdate_long_term_memory accept evicted entries parameter - process_step and aprocess_step pass evicted entries to the consolidation methods - Updated existing tests and added regression test verifying the LLM receives the evicted entries
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #166 +/- ##
==========================================
+ Coverage 90.08% 90.85% +0.76%
==========================================
Files 19 19
Lines 1503 1542 +39
==========================================
+ Hits 1354 1401 +47
+ Misses 149 141 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Cover _aupdate_long_term_memory to ensure the async consolidation path receives evicted entries and stores the LLM response correctly.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Fixes #107
STLTMemory._process_step_core()discards popped short-term entries without passing them to the LLM for summarization. Instead,_build_consolidation_prompt()formats the remaining active short-term memories — so the LLM redundantly summarizes memories that are still in short-term, while the evicted ones are permanently lost.Root Cause
self.short_term_memory.popleft()discards the entry (return value is ignored)consolidation_capacitycould be > 1_build_consolidation_prompt()sends the remaining short-term memories to the LLM — the evicted entries are never seenChanges
_process_step_core(): Now captures all evicted entries (up toconsolidation_capacity) in a list and returns them alongside the new entry_build_consolidation_prompt(evicted_entries): Accepts the evicted entries and constructs a prompt asking the LLM to integrate them into the existing long-term summary_update_long_term_memory(evicted_entries)/_aupdate_long_term_memory(evicted_entries): Updated signatures to receive and forward evicted entriesprocess_step()/aprocess_step(): Pass evicted entries to the consolidation methodstest_update_long_term_memoryandtest_long_term_memory_stores_string_not_response_objectto match new signatures; addedtest_consolidation_receives_evicted_entriesregression testBefore vs After
consolidation_capacityentriesTest Plan